minor: Add segment debug logging to CachingClusteredClient#19648
minor: Add segment debug logging to CachingClusteredClient#19648cecemei wants to merge 5 commits into
Conversation
This reverts commit e15f3bc.
FrankChen021
left a comment
FrankChen021
left a comment
There was a problem hiding this comment.
I have reviewed the code for correctness, edge cases, concurrency, and integration risks; no issues found.
Reviewed 1 of 1 changed files.
This is an automated review by Codex GPT-5.5
FrankChen021
left a comment
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
Reviewed 1 of 1 changed files.
Found 1 issue: debug logging can emit synthetic segment IDs for sliced timeline lookups.
This is an automated review by Codex GPT-5.5
| segmentServers.stream().map( | ||
| s -> SegmentId.of( | ||
| dataSource, | ||
| s.getSegmentDescriptor().getInterval(), |
There was a problem hiding this comment.
[P2] Log real segment IDs, not query slices
The debug log rebuilds a SegmentId from SegmentDescriptor.getInterval(), but computeSegmentsToQuery fills that descriptor from TimelineObjectHolder.getInterval(). VersionedIntervalTimeline.lookup trims the first or last holder to the requested interval, so a query over one day inside a week-long segment logs a synthetic segment id that never existed. Use s.getServer().getSegment().getId() for the segment id, and log the query interval separately if needed.
4 participants
Description
When debugging slow or incorrect queries on the broker, it's useful to know how many segments were found during timeline lookup and how they break down by interval, version, and server type (realtime vs historical).
This PR has: